Skip to content

Conversation

matthewdale
Copy link
Collaborator

@matthewdale matthewdale commented May 25, 2024

GODRIVER-3240

Summary

  • Use functions from the "encoding/binary" package instead of custom bit-shifting logic to convert between binary and integer values in the wiremessage package.
  • Add unit tests for the modified wiremessage functions.

Background & Motivation

@mongodb-drivers-pr-bot mongodb-drivers-pr-bot bot added the review-priority-low Low Priority PR for Review: within 3 business days label May 25, 2024
Copy link
Contributor

API Change Report

No changes found!

@matthewdale matthewdale force-pushed the experiment-use-binary-in-wm branch from b212180 to f5b7c10 Compare June 5, 2024 23:51
@matthewdale matthewdale marked this pull request as ready for review June 6, 2024 02:50
@matthewdale matthewdale requested a review from blink1073 June 6, 2024 16:51
@matthewdale matthewdale changed the title Replace all bit-shifting in wiremessage with encoding/binary calls. GODRIVER-3240 Replace all bit-shifting in wiremessage with encoding/binary calls. Jun 6, 2024
Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

return nil, errors.New("malformed wire message: insufficient bytes to read single document")
}
case wiremessage.DocumentSequence:
// TODO(GODRIVER-617): Implement document sequence returns.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment appears stale and references a GODRIVER ticket that is "Done", so I removed it.

@matthewdale matthewdale merged commit ab38b74 into mongodb:master Jun 8, 2024
blink1073 pushed a commit to blink1073/mongo-go-driver that referenced this pull request Jun 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-priority-low Low Priority PR for Review: within 3 business days
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants